-
Notifications
You must be signed in to change notification settings - Fork 274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(Rebased) Pr/behind proxy middleware #1660
base: main
Are you sure you want to change the base?
Conversation
Uses existing plack middlewares (ReverseProxy and ReverseProxyPath) to handle all request header modification when operating behing a proxy. Allows for variants (eg HTTP_X_FORWARDED_PROTOCOL) supported by Dancer(2) but not from those existing ReverseProxy middleware. Special cases REQUEST_BASE header to work with ReverseProxyPath so proxies from/to non-root paths "just work"(tm). (Alternate to #571.) As a middleware, devs get more flexability as to where to apply it; they can use Plack::Builder to wrap this around their app as well as further path/header altering middleware. This could be released as a seperate package; its not Dancer2 specific.
set behind_proxy => 1; We now 'use' the middleware directly rather than letting Plack::Builder load it for us. May allow earlier version of Plack to be used as a dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like how this came together. We removed a test in t/request.t
and we don't have any tests for lib/Dancer2/Middleware/BehindProxy.pm
. Asking here so we can capture this for our future selves: do we not need to test this going forward because of the changes, or should we re-implement the test we removed in another fashion? That's about my only concern moving forward with this.
@@ -0,0 +1,81 @@ | |||
use strict; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this test file is supposed to be part of this PR. It's causing the Appveyor tests to fail.
9d4d838
to
2faca00
Compare
@@ -0,0 +1,58 @@ | |||
package Dancer2::Middleware::BehindProxy; | |||
# ABSTRACT: Support Dancer2 apps when operating behing a reverse proxy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: behind
I still think this is a better way to handle the psgi It's taken some time, but I concur with the comments on the original PR about the namespace; something like |
This is a rebased branch of #590. :)
It's only for review against the original PR. Please do not merge it.